-
Notifications
You must be signed in to change notification settings - Fork 3.4k
HBASE-29871 During WAL write times out add DataNode address in. the exception message #7719
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
| if (evt instanceof IdleStateEvent && ((IdleStateEvent) evt).state() == READER_IDLE) { | ||
| promise | ||
| .tryFailure(new IOException("Timeout(" + timeoutMs + "ms) waiting for response")); | ||
| promise.tryFailure(new IOException( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@srinireddy2020
In other two places we are logging "channel.remoteAddress" in message and here dnInfo.toString.
Can't we keep it consistent?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the method parameter already having the DN details so no need to call channel.remoteAddress.
pankaj72981
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM +1
|
Please fix the spotless issue? |
f7102a3 to
435757a
Compare
Spot less fixed. please check |
| } | ||
| String logInfo = "ack with firstBadLink as " + resp.getFirstBadLink(); | ||
| String logInfo = | ||
| "ack with firstBadLink as " + resp.getFirstBadLink() + " from datanode " + dnInfo; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks good. Just some minor nits.
In a previous change, we print both the hostname and IP when printing datanode info, details see: #6148
It might be better to follow that same pattern here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comment addressed, please check.
For better debug, add datanode address to the exception message during WAL write times out